Add thread-safety documentation and -O3 optimization#3
Conversation
olaugh
commented
Jan 20, 2026
olaugh
commented
- Document that Genesis Plus GX is not thread-safe due to global state
- Add examples of safe (fork-based) and unsafe (thread-based) parallelism
- Enable -O3 optimization for faster test execution
- Document that Genesis Plus GX is not thread-safe due to global state - Add examples of safe (fork-based) and unsafe (thread-based) parallelism - Enable -O3 optimization for faster test execution Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive documentation about thread-safety concerns in Genesis Plus GX and enables -O3 compiler optimization for faster test execution. The documentation clearly warns users that the emulator uses global state and cannot be safely used with thread-based parallelism.
Changes:
- Added extensive thread-safety warnings and documentation with examples of safe (fork-based) and unsafe (thread-based) parallelism patterns
- Enhanced class-level documentation for the Emulator class to emphasize non-thread-safe behavior
- Enabled -O3 optimization flag for the Genesis Plus GX core library to improve test execution performance
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| include/gxtest.h | Added detailed thread-safety documentation with fork-based and thread-based examples, and enhanced Emulator class documentation |
| BUILD.bazel | Added -O3 compiler optimization flag to improve emulator performance during test execution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "vendor/genplusgx/cd_hw/*.h", | ||
| ]) + ["src/osd.h"], | ||
| copts = [ | ||
| "-O3", # Always optimize emulator for fast test execution |
There was a problem hiding this comment.
The -O3 optimization flag is set unconditionally with the comment "Always optimize emulator for fast test execution". While this is beneficial for test execution speed, it may hinder debugging efforts when issues arise in the emulator core. Consider whether this should respect the build configuration mode (e.g., using select() to apply -O3 only in optimized builds, or opt builds) to preserve debuggability in debug configurations.
include/gxtest.h
Outdated
| * if (pid == 0) { | ||
| * // Child process - has its own copy of global state | ||
| * GX::Emulator emu; | ||
| * emu.LoadRom("game.bin"); | ||
| * emu.RunFrames(1000); | ||
| * // Write results to pipe, exit |
There was a problem hiding this comment.
The fork-based example is incomplete and lacks proper error handling. The child process should explicitly call exit() or _exit() after completing its work to prevent it from continuing into the parent's code path. Additionally, the example doesn't show error handling for fork() failures (when it returns -1). Consider adding "exit(0);" after the "Write results to pipe" comment and showing error handling for the fork() call to make this a more complete and accurate example.
| * if (pid == 0) { | |
| * // Child process - has its own copy of global state | |
| * GX::Emulator emu; | |
| * emu.LoadRom("game.bin"); | |
| * emu.RunFrames(1000); | |
| * // Write results to pipe, exit | |
| * if (pid < 0) { | |
| * // Handle fork() error | |
| * perror("fork"); | |
| * exit(1); | |
| * } else if (pid == 0) { | |
| * // Child process - has its own copy of global state | |
| * GX::Emulator emu; | |
| * emu.LoadRom("game.bin"); | |
| * emu.RunFrames(1000); | |
| * // Write results to pipe, then exit child | |
| * exit(0); |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * } | ||
| * // Parent collects results from children |
There was a problem hiding this comment.
The fork-based example is incomplete and may mislead developers. It's missing several critical elements:
- The child process should call exit() or _exit() to prevent it from continuing execution after the emulator work
- The parent process should wait for the child using waitpid() or similar to prevent zombie processes
- Error handling for fork() failure (when it returns -1) is not shown
Consider adding a complete example that shows proper process management, or add a comment noting that the example is simplified and production code should include proper error handling and process cleanup.
| * } | |
| * // Parent collects results from children | |
| * } else { | |
| * int status = 0; | |
| * if (waitpid(pid, &status, 0) < 0) { | |
| * perror("waitpid"); // Handle waitpid() error | |
| * } | |
| * // Parent collects results from children | |
| * } |
- Add profiler.h/cpp with per-function cycle counting - Enable HOOK_CPU to hook into Genesis Plus GX instruction execution - Support Simple mode (fast) and CallStack mode (inclusive cycles) - Load symbols from ELF files via nm - Fix cpuhook.h extern declaration for cpu_hook Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
